Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduced search support for multiple programming languages #116

Merged

Conversation

dzmitry-kankalovich
Copy link
Contributor

Checklist

  • My code is properly formatted using the latest black
  • I have added/updated tests if needed
  • I have tried running my code manually
  • I have checked for spelling errors

Description

Similar to #108 implementation of multiple programming languages support. The reason for another changeset is that the original PR seems to be stale. This one is up to date + fixed/added tests.

@@ -19,12 +20,19 @@


# could be made into config option in the future
CACHED_RESULT_PATH = xdg_cache_home() / "starcli.json"
CACHED_RESULT_PATH = Path(BaseDirectory.xdg_cache_home) / "starcli.json"
Copy link
Contributor Author

@dzmitry-kankalovich dzmitry-kankalovich Dec 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is weird, but basically doing pip3 install -r requirements_dev.txt installs a version of xdg that does not export xdg_cache_home as is. Possibly a breaking change in some of the minor/patch version of xdg? I've double-checked it with fresh venv - same problem.

Copy link
Owner

@hedyhli hedyhli Jan 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's weird, I've tested with python 3.8 and 3.10, on both ubuntu and arch. Are you having this issue by installing from PyPI as well?

What is your OS and python version?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, it seems like you might be using pyxdg, as xdg doesn't have BaseDirectory.

@dzmitry-kankalovich dzmitry-kankalovich mentioned this pull request Dec 22, 2022
4 tasks
@hedyhli
Copy link
Owner

hedyhli commented Dec 29, 2022

Thanks for the PR! I'll take a look when I'm free.

Copy link
Owner

@hedyhli hedyhli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for reviving the feature! Here are a few comments after a quick look.

starcli/__main__.py Outdated Show resolved Hide resolved
starcli/search.py Show resolved Hide resolved
@hedyhli hedyhli merged commit 0f5cfa5 into hedyhli:main Jan 11, 2023
@hedyhli
Copy link
Owner

hedyhli commented Jan 11, 2023

Hi @dzmitry-kankalovich. This has been merged without the xdg.BaseDirectory change. If you're still having issues with it, please open a separate issue and I'll be happy to help debug it further. 🙂

@all-contributors please add @dzmitry-kankalovich for code.

@allcontributors
Copy link
Contributor

@hedyhli

I've put up a pull request to add @dzmitry-kankalovich! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants